Skip to content

fix(bigtable): respect query limit smaller than pageSize and avoid mutating Query in QueryPaginator - #14821

Merged
igorbernstein2 merged 1 commit into
googleapis:mainfrom
igorbernstein2:b573787373
Oct 28, 2026
Merged

igorbernstein2 merged 1 commit into
googleapis:mainfrom
igorbernstein2:b573787373

Conversation

@igorbernstein2

@igorbernstein2 igorbernstein2 commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Fixes two issues in Query.QueryPaginator:

  • Clamps the initial page limit to Math.min(pageSize, remainingRows) when the query has an overall row limit smaller than pageSize (matching QueryPaginator#advance).
  • Clones the input Query so createPaginator and advance do not mutate the caller's Query instance in-place.

The primary issue is that when the paginated query has a limit smaller than the configured page size of the paginator, the query limit incorrectly gets expanded to the page size

@igorbernstein2
igorbernstein2 requested review from a team as code owners October 28, 2026 20:35

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the QueryPaginator constructor in Query.java to clone the query object and properly adjust its limit, preventing unintended mutations to the original query. It also adds corresponding unit tests in QueryTest.java to verify that the original query remains unchanged and to test scenarios where the limit is smaller than the chunk size. The feedback suggests simplifying the conditional logic in the constructor using a ternary operator to reduce code duplication and improve readability.

@igorbernstein2
igorbernstein2 enabled auto-merge (squash) October 28, 2026 20:38
@igorbernstein2
igorbernstein2 merged commit cecd09e into googleapis:main Oct 28, 2026
202 of 203 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants